[#950] Announce a ReplicaOfflineMsg before it is published, not after it may have been forwarded - #978

Rebased onto master now that #946 has landed. The conflict both PRs predicted is resolved and the description above is updated to match; no review had been posted yet, so nothing here answers a review comment.

The two changes met in the same three files.

LDAPReplicationDomain.publishReplicaOfflineMsg() — #946 wrapped the announcement in if (offlineCSN != null). With the announcement moved to the publish site there is nothing left to guard, so the method keeps only the trace #946 added for the message a change in flight held back:

final CSN offlineCSN = pendingChanges.putReplicaOfflineMsg();
if (offlineCSN == null && logger.isTraceEnabled())
{
  /*
   * The announcement itself is made where the message is published, so nothing has to be
   * reported here: a message which never reached the wire was never announced either.
   */
  logger.trace("Replica " + getServerId() + " of domain baseDN=" + getBaseDN()
      + " could not announce itself offline: a change which is still in flight holds"
      + " the message back, and " + pendingChanges.size() + " change(s) are pending");
}

PendingChanges.java — merged without a conflict, and both halves stand: putReplicaOfflineMsg() still gives up on the message which stayed queued and returns null, while pushCommittedChanges() announces before domain.publish(msg).

PendingChangesTest.java — an add/add conflict, now the union of both files, five cases. The three #946 added are byte-for-byte unchanged; only the newPendingChanges() helper grew the announcer, behind an overload which keeps those three calling it with one argument.

One case needed more than a merge, and it is worth naming. theReplicaOfflineMsgHeldBackByAChangeInFlightIsAnnouncedOnlyWhenItIsPublished asserted that a held-back message is announced once the change in flight lets it out. After #946 there is no such message left to let out - putReplicaOfflineMsg() removes it from the queue rather than leaving it there - so the assertion contradicted master. It is now theReplicaOfflineMsgHeldBackByAChangeInFlightIsNeverAnnounced, and pins both points: nothing announced while the change is in flight, nothing announced when it completes either.

PendingChangesTest, DSRSShutdownSyncTest and ReplicationServerShutdownSyncTest: 25 tests, all green. The regression the first new case exists for is still caught - putting announce() back behind domain.publish() fails it on "the message was forwarded, so nothing must hold the shutdown back any longer".

For the record, since the run on the pre-rebase head 8175d6e went red:
DependencyTest.addModDelDependencyTest failed there, and it does not look like this change.

The map this branch moves the write of - DSRSShutdownSync.replicaOfflineMsgs - is read only
by ReplicationServer.shutdown() and by the non-DS branch of ServerWriter. That test has a
single replication server and only DS handlers, and its replication server is shut down in the
finally after the assertion, so neither reader runs before it. Ten local runs of
DependencyTest, five with this patch and five without, were green at 5.36-5.48 s against the
30 s budget the failure exhausted.

It looks like #924; the evidence, and a second sighting of the same signature on another
branch, are in
#924 (comment).

Rebased onto master (36d4af9bd7) to pick up the fix for #924. Same single commit, now ec70866ee6,
and git log origin/master..HEAD shows only it. No code of this branch moved: the diff against
master is the same 128 added and 15 removed lines in the same three files.

Only LDAPReplicationDomain.java was touched by both sides, and in different places - #971 rewrote
enable(), this branch moves the announcement out of publishReplicaOfflineMsg() and into
PendingChanges.pushCommittedChanges(). Nothing to reconcile beyond the merge.

This finishes the note above about the red run on the pre-rebase head. That failure now has a name:
it is #924, and the fix for it reached master on 2026-09-09 13:44 UTC, after this branch was cut from
2a7bb9d7ed. What settles it is the logs/access of that job - note that it is
attempt 1 of run
34316279431 (job 102353062700) which holds it, since the job id now serves the cancelled re-run. The
last operation logged there is the MODIFY dn="o=test" which saves the ServerState right after
enable(), at 06:45:06, and then nothing at all for the 30 s the test waits. A delivery which arrives
in that window is given up on and never asked for again, which is exactly what #971 fixes. The ten
local runs reported above were green because the window is narrow, not because the test is unaffected.

Verified on the rebased branch rather than on the old head:

• opendj-server-legacy test-compiles.
• PendingChangesTest 5, DSRSShutdownSyncTest 12, ReplicationServerShutdownSyncTest 8 - 25
  tests, no failures.

Rebased onto master (13d57e063c), where #976 and #947 have landed. Still one commit, now
4aeb3b35d2; git log origin/master..HEAD shows only it. No review had been posted, so nothing
here answers one.

The conflict was with #976, in the ReplicaOfflineMsg branch of pushCommittedChanges(): master
now reads the answer of domain.publish(msg), this branch announces before that call. Taking both
is not enough - an announcement made before a publish the broker then refuses is exactly the stale
record #976 removed, back in a new shape. So the resolution is the one the description of #950
proposed, and what this PR had listed under "not fixed here":

final CSN offlineCSN = msg.getCSN();
replicaOfflineAnnouncer.announce(offlineCSN);
if (domain.publish(msg))
{
  publishedOfflineCSN = offlineCSN;
}
else
{
  // The broker wrote it to no session, so nobody will forward what was announced.
  replicaOfflineAnnouncer.withdraw(offlineCSN);
}

ReplicaOfflineAnnouncer grew withdraw(), and DSRSShutdownSync a matching
replicaOfflineMsgNotSent(): it removes only the entry carrying that CSN - the two-argument
remove() the forward guard already uses - and wakes the shutdown up the way a forward does.
putReplicaOfflineMsg(), its verdict and the trace of publishReplicaOfflineMsg() are as #976 left
them; only the comment above the trace now names the withdrawal.

#947 changed replicaOfflineMsgForwarded() to take the peer id, so the forward the test reports
from inside publish() names one; with no peer recorded the first forward still ends the wait,
which is the fallback #947 kept for exactly this shape. The other case that fallback's comment
named - an announcement recorded after its message was relayed - no longer exists, and the comment
no longer says it does.

Tests, on the rebased head:

• PendingChangesTest is the union of both sides plus one case, 8 in all. New here:
  theAnnouncementOfAReplicaOfflineMsgTheBrokerRefusedIsWithdrawn checks from inside publish()
  that the announcement is in place, refuses the message, and asserts nothing holds the shutdown
  back afterwards.
• DSRSShutdownSyncTest grows three cases for the withdrawal: it ends the wait, it wakes a waiting
  shutdown up, and the withdrawal of an earlier announcement leaves a newer one of the same replica
  alone.
• PendingChangesTest 8, DSRSShutdownSyncTest 25, ReplicationServerShutdownSyncTest 13 - 46
  tests, no failures, -Pprecommit checkstyle included.

Both regressions were watched: with the announcement moved back behind domain.publish(),
theReplicaOfflineMsgIsAnnouncedBeforeItIsPublished fails on "the message was forwarded, so
nothing must hold the shutdown back any longer"; with the withdraw() call removed,
theAnnouncementOfAReplicaOfflineMsgTheBrokerRefusedIsWithdrawn fails on "the message never
reached the wire, so nothing must hold the shutdown back".

The description above is updated to match.

Round 1 addressed in fba06c0ee9; rebased onto master (cebef54070, where #958, #964 and #974 have
landed - none of them touches this code, the merge was clean). Still one commit,
git log origin/master..HEAD shows only it.

Blocking 1 - a withdrawal emptied a slot announce() had re-used. Confirmed as described:
the second disableService() announces CSN2 over CSN1, the broker refuses it on connectionError
or connectRequiresRecovery, and withdraw(CSN2) removed the slot CSN1 was still waiting in - a
regression against BASE, where a refused CSN2 was never announced. Fixed in the shape suggested:
PendingOfflineMsg keeps the announcement it displaced (compute() in replicaOfflineMsgSent),
and replicaOfflineMsgNotSent puts it back with the three-argument replace(), or removes the
entry when there was none. theWithdrawalOfALaterMessageGivesTheEarlierOneItsWaitBack is in, as
proposed, and was watched failing first: "the earlier message went out and nobody has forwarded
it yet - expected false but was true".

One residue is named in the javadoc rather than handled: while CSN2 stands in CSN1's place,
whatever is reported about CSN1 - a forward, a peer going away - is not seen by it, and after the
restore the shutdown waits out what is left of CSN1's own grace period. The window is the one
refused publish (immediate on connectionError / connectRequiresRecovery; up to the reconnect
when the session is null), and the cost is bounded by a grace period which is already running.
Walking the displaced chain from the forward guard would close it, but I would rather not add
that for a window this narrow unless you see it differently.

Blocking 2 - nothing pinned that the announcement of a published message stands. Confirmed
by running the mutant: withdraw on both arms of if (domain.publish(msg)) was green 33/33.
theAnnouncementOfAPublishedMessageStandsUntilItIsForwarded is in, and forwardWhilePublishing
asserts from inside the publish that the announcement is already there. Re-run:

• withdraw on both arms - dies on theAnnouncementOfAPublishedMessageStandsUntilItIsForwarded;
• announcement moved back behind domain.publish() - theReplicaOfflineMsgIsAnnouncedBeforeItIsPublished
  now dies on its own name, "the message must be announced before it is published", as does
  theAnnouncementOfAReplicaOfflineMsgTheBrokerRefusedIsWithdrawn.

Production announcer untested. ShutdownSyncAnnouncer(DSRSShutdownSync, DN) is a
package-private class now, and PendingChangesTest.newPendingChanges() builds its pending
changes with it rather than with an announcer of its own - so all nine cases go through the
production code, and the swap of replicaOfflineMsgSent / replicaOfflineMsgNotSent dies on
three of them. That seemed better than one dedicated case next to a duplicate.

theWaitEndsWhenTheMessageIsWithdrawn. canShutdown().isTrue() added. One note on the
rationale: a withdrawal which notified without emptying the slot would have failed this case
already, on elapsed < LONG_GRACE_PERIOD - awaitReplicaOfflineMsgsForwarded() re-reads the
remaining grace period after each wake-up and goes back to waiting - but only after the 60 s;
the added assertion makes it immediate and says why.

"Another thread" / "a newer one made in the meantime". Both reworded. The test's javadoc now
says what it pins - a stale withdrawal, of a message the replica has since announced again, is
ignored - and the replicaOfflineMsgNotSent javadoc describes the displaced announcement
instead.

:49 - "announced".

"What really went out, and nothing else". The sentence was in the ReplicaOfflineAnnouncer
javadoc and the commit message rather than the description; all three now say "what the broker
reports as written", and the description names the two silent returns of Session.publish()
as #976's contract, unchanged here.

Tests, on the rebased head, class per JVM: PendingChangesTest 9, DSRSShutdownSyncTest 26,
ReplicationServerShutdownSyncTest 13 - 48, no failures; -Pprecommit reactor and javadoc doclint
green. The description above is updated to match.

Round 2 addressed in 43f3d15faf; rebased onto master (fef4292a5f, where #977, #1033 and #1047 have
landed - #977 rewrites other parts of LDAPReplicationDomain, the merge was clean). Still one
commit, git log origin/master..HEAD shows only it.

The restore case pinned the CSN only. Confirmed by running the mutant: with the displaced entry
rebuilt - new PendingOfflineMsg(displaced.csn, System.nanoTime(), null) - the two cases below are
the only ones which fail, 27 of 29 stay green, so nothing else in the class was looking.
theRestoredAnnouncementIsStillOwedTheForwardsItWasQueuedFor is in as proposed, and the
grace-period half too, as
theRestoredAnnouncementKeepsWhatIsLeftOfItsOwnGracePeriod: GRACE_PERIOD, Sent(1), 300 ms,
Sent(2), NotSent(2), 250 ms more, canShutdown true - a whole new period would still be running.
The second one is there because the first alone does not pin the clock: a mutant which copies the
peers over and resets only sentTime survives it, and dies on the second - measured, both ways.

The stale-withdrawal case no longer pinned the guard. Confirmed: with csn.equals dropped,
NotSent(1) restored CSN 1 behind the newer entry and the case stayed green. The forward of the
withdrawn CSN is in, with the assertion text you proposed; the mutant dies on it.

The residue's other end. Taken: replicaOfflineMsgNotSent gives the displaced announcement its
place back only while !isFullyForwarded() - awaitedForwarders known and empty - and removes
the entry otherwise, with the race named at the condition. No case for it, and I would rather say
so than pretend: the displaced entry's set can only empty through the forward guard, which reads
the top entry, so a test would have to land the announce between its get and its remove.
"Restore unconditionally" therefore survives, by construction.

The chain is never pruned. Taken, in the shape you gave: compute() keeps the displaced entry
only while gracePeriodLeft(displaced, announcedAt) > 0, and remainingGracePeriod reads the
same helper. The chain is reachable only through the top entry, so the first announcement made
more than a grace period after the previous one drops the whole of it. Worth adding to your list
of producers: restartSession() of #974 announces again every 1-10 s for as long as a replay keeps
failing, so on a replica whose message nobody in this process forwards the chain was not a
one-off per config change but a steady drip. Retention only, so "keep unconditionally" survives
the suite, and the description says so.

The disclosed residue has no case. aForwardReportedWhileARefusedAnnouncementStoodIsNotSeen
is in as proposed. It stays false with the isFullyForwarded() check above, as it should: a
forward of CSN 1 while CSN 2 stands in front never reaches CSN 1's set.

"The one publish the broker refuses". The javadoc now says where the window ends: at once on a
connection error or a pending recovery, the broker's retry loop up to the reconnect when it has no
session.

A Dispatched dropped behind the refused entry. The javadoc names it, with its opposite
consequence: the restored announcement keeps no recipients, and the first forward ends its wait
as for a message no peer was recorded for. The one-node descent is left out for a window the RS
reader thread closes right after the socket read.

The description's road. Rewritten: the roads are the fractional/assured config change,
restartService(), the disable()/enable() pair of a total update and restartSession();
nothing calls enableService() after shutdown().

Tests, on the rebased head, class per JVM: PendingChangesTest 9, DSRSShutdownSyncTest 29,
ReplicationServerShutdownSyncTest 13 - 51, no failures; -Pprecommit reactor and javadoc
doclint green. Mutants re-run on this head, each dying where its name says:

• rebuild the displaced entry - theRestoredAnnouncementIsStillOwedTheForwardsItWasQueuedFor and
  theRestoredAnnouncementKeepsWhatIsLeftOfItsOwnGracePeriod;
• reset only its clock - theRestoredAnnouncementKeepsWhatIsLeftOfItsOwnGracePeriod;
• drop the csn.equals guard - theWithdrawalOfAnEarlierMessageLeavesANewerOneAlone, on "the
  stale withdrawal was ignored, not turned into a restore";
• the three of the earlier rounds - announce behind the publish, withdraw on both arms, the swap in
  ShutdownSyncAnnouncer - die on the same cases as before.

The description above is updated to match.